From bc0e3e505e7c580018b72006440ee42d5f0ab910 Mon Sep 17 00:00:00 2001 From: Enrico Risa Date: Wed, 17 Jul 2024 16:20:20 +0200 Subject: [PATCH 1/2] feat: enable proof of possession check on token validation --- .../core/CoreServicesExtension.java | 2 +- .../verification/AccessTokenVerifierImpl.java | 8 ++---- .../AccessTokenVerifierImplComponentTest.java | 11 +++----- .../AccessTokenVerifierImplTest.java | 7 +---- .../tests/PresentationApiEndToEndTest.java | 26 ++++++++++++++++++- 5 files changed, 33 insertions(+), 21 deletions(-) diff --git a/core/identity-hub-core/src/main/java/org/eclipse/edc/identityhub/core/CoreServicesExtension.java b/core/identity-hub-core/src/main/java/org/eclipse/edc/identityhub/core/CoreServicesExtension.java index bd1a6f153..e8d9f43d7 100644 --- a/core/identity-hub-core/src/main/java/org/eclipse/edc/identityhub/core/CoreServicesExtension.java +++ b/core/identity-hub-core/src/main/java/org/eclipse/edc/identityhub/core/CoreServicesExtension.java @@ -136,7 +136,7 @@ public void initialize(ServiceExtensionContext context) { @Provider public AccessTokenVerifier createAccessTokenVerifier(ServiceExtensionContext context) { var keyResolver = new KeyPairResourcePublicKeyResolver(store, keyParserRegistry, context.getMonitor(), fallbackService); - return new AccessTokenVerifierImpl(tokenValidationService, keyResolver, tokenValidationRulesRegistry, context.getMonitor(), publicKeyResolver, participantContextService); + return new AccessTokenVerifierImpl(tokenValidationService, keyResolver, tokenValidationRulesRegistry, publicKeyResolver, participantContextService); } @Provider diff --git a/core/lib/accesstoken-lib/src/main/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImpl.java b/core/lib/accesstoken-lib/src/main/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImpl.java index f78b6314a..c20178c4e 100644 --- a/core/lib/accesstoken-lib/src/main/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImpl.java +++ b/core/lib/accesstoken-lib/src/main/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImpl.java @@ -19,7 +19,6 @@ import org.eclipse.edc.identityhub.spi.verification.AccessTokenVerifier; import org.eclipse.edc.jwt.spi.JwtRegisteredClaimNames; import org.eclipse.edc.keys.spi.PublicKeyResolver; -import org.eclipse.edc.spi.monitor.Monitor; import org.eclipse.edc.spi.result.Result; import org.eclipse.edc.token.spi.TokenValidationRule; import org.eclipse.edc.token.spi.TokenValidationRulesRegistry; @@ -46,16 +45,14 @@ public class AccessTokenVerifierImpl implements AccessTokenVerifier { private final TokenValidationService tokenValidationService; private final KeyPairResourcePublicKeyResolver localPublicKeyService; private final TokenValidationRulesRegistry tokenValidationRulesRegistry; - private final Monitor monitor; private final PublicKeyResolver publicKeyResolver; private final ParticipantContextService participantContextService; - public AccessTokenVerifierImpl(TokenValidationService tokenValidationService, KeyPairResourcePublicKeyResolver localPublicKeyService, TokenValidationRulesRegistry tokenValidationRulesRegistry, Monitor monitor, + public AccessTokenVerifierImpl(TokenValidationService tokenValidationService, KeyPairResourcePublicKeyResolver localPublicKeyService, TokenValidationRulesRegistry tokenValidationRulesRegistry, PublicKeyResolver publicKeyResolver, ParticipantContextService participantContextService) { this.tokenValidationService = tokenValidationService; this.localPublicKeyService = localPublicKeyService; this.tokenValidationRulesRegistry = tokenValidationRulesRegistry; - this.monitor = monitor; this.publicKeyResolver = publicKeyResolver; this.participantContextService = participantContextService; } @@ -92,8 +89,7 @@ public Result> verify(String token, String participantId) { var atSub = at.getStringClaim(JwtRegisteredClaimNames.SUBJECT); // correlate sub and access_token.sub if (!Objects.equals(subClaim, atSub)) { - monitor.warning("ID token [sub] claim is not equal to [%s.sub] claim: expected '%s', got '%s'. Proof-of-possession could not be established!".formatted(TOKEN_CLAIM, subClaim, atSub)); - // return failure("ID token 'sub' claim is not equal to '%s.sub' claim.".formatted(ACCES_TOKEN_CLAIM)); + return Result.failure("ID token [sub] claim is not equal to [%s.sub] claim: expected '%s', got '%s'.".formatted(TOKEN_CLAIM, subClaim, atSub)); } return Result.success(); }; diff --git a/core/lib/accesstoken-lib/src/test/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImplComponentTest.java b/core/lib/accesstoken-lib/src/test/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImplComponentTest.java index 28fdc9bf5..4130978b6 100644 --- a/core/lib/accesstoken-lib/src/test/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImplComponentTest.java +++ b/core/lib/accesstoken-lib/src/test/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImplComponentTest.java @@ -26,7 +26,6 @@ import org.eclipse.edc.identityhub.spi.participantcontext.ParticipantContextService; import org.eclipse.edc.identityhub.spi.participantcontext.model.ParticipantContext; import org.eclipse.edc.junit.annotations.ComponentTest; -import org.eclipse.edc.spi.monitor.Monitor; import org.eclipse.edc.spi.result.Result; import org.eclipse.edc.spi.result.ServiceResult; import org.eclipse.edc.token.TokenValidationRulesRegistryImpl; @@ -49,9 +48,7 @@ import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.startsWith; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ComponentTest @@ -60,7 +57,6 @@ class AccessTokenVerifierImplComponentTest { public static final String STS_PUBLIC_KEY_ID = "sts-key-123"; public static final String PARTICIPANT_CONTEXT_ID = "test_participant"; public static final String PARTICIPANT_DID = "did:web:test_participant"; - private final Monitor monitor = mock(); private final ParticipantContextService participantContextService = mock(); private AccessTokenVerifierImpl verifier; private KeyPair stsKeyPair; // this is used to sign the acces token @@ -88,7 +84,7 @@ void setUp() throws NoSuchAlgorithmException, InvalidAlgorithmParameterException when(resolverMock.resolveKey(anyString(), anyString())).thenReturn(Result.success(stsKeyPair.getPublic())); when(participantContextService.getParticipantContext(anyString())).thenReturn(ServiceResult.success(ParticipantContext.Builder.newInstance().did(PARTICIPANT_DID).participantId(PARTICIPANT_CONTEXT_ID).apiTokenAlias("foobar").build())); - verifier = new AccessTokenVerifierImpl(tokenValidationService, resolverMock, ruleRegistry, monitor, (id) -> Result.success(providerKeyPair.getPublic()), participantContextService); + verifier = new AccessTokenVerifierImpl(tokenValidationService, resolverMock, ruleRegistry, (id) -> Result.success(providerKeyPair.getPublic()), participantContextService); } @Test @@ -201,8 +197,9 @@ void assertWarning_whenSubjectClaimsMismatch() { .build()); var siToken = createSignedJwt(providerKeyPair.getPrivate(), new JWTClaimsSet.Builder().claim("token", accessToken).subject("mismatching-subject").build()); - assertThat(verifier.verify(siToken, PARTICIPANT_CONTEXT_ID)).isSucceeded(); - verify(monitor).warning(startsWith("ID token [sub] claim is not equal to [token.sub]")); + assertThat(verifier.verify(siToken, PARTICIPANT_CONTEXT_ID)).isFailed() + .detail() + .startsWith("ID token [sub] claim is not equal to [token.sub] claim"); } diff --git a/core/lib/accesstoken-lib/src/test/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImplTest.java b/core/lib/accesstoken-lib/src/test/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImplTest.java index 3314a1ed4..9d885bedf 100644 --- a/core/lib/accesstoken-lib/src/test/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImplTest.java +++ b/core/lib/accesstoken-lib/src/test/java/org/eclipse/edc/identityhub/accesstoken/verification/AccessTokenVerifierImplTest.java @@ -50,7 +50,7 @@ class AccessTokenVerifierImplTest { .build(); private final KeyPairResourcePublicKeyResolver localPublicKeyResolver = mock(); private final ParticipantContextService participantContextService = mock(); - private final AccessTokenVerifierImpl verifier = new AccessTokenVerifierImpl(tokenValidationSerivce, localPublicKeyResolver, tokenValidationRulesRegistry, mock(), pkResolver, participantContextService); + private final AccessTokenVerifierImpl verifier = new AccessTokenVerifierImpl(tokenValidationSerivce, localPublicKeyResolver, tokenValidationRulesRegistry, pkResolver, participantContextService); @Test void verify_validSiToken_validAccessToken() { @@ -92,11 +92,6 @@ void verify_accessTokenValidationFails() { .detail().isEqualTo("test-failure"); } - @Test - void verify_accessTokenSubNotEqualToSub_shouldFail() { - - } - @Test void verify_accessTokenDoesNotContainScopeClaim() { var accessToken = JwtCreationUtil.generateJwt(OWN_DID, OWN_DID, OTHER_PARTICIPANT_DID, Map.of(/*scope missing*/), JwtCreationUtil.CONSUMER_KEY); diff --git a/e2e-tests/api-tests/src/test/java/org/eclipse/edc/identityhub/tests/PresentationApiEndToEndTest.java b/e2e-tests/api-tests/src/test/java/org/eclipse/edc/identityhub/tests/PresentationApiEndToEndTest.java index a4a5a2a2a..bfa8277fc 100644 --- a/e2e-tests/api-tests/src/test/java/org/eclipse/edc/identityhub/tests/PresentationApiEndToEndTest.java +++ b/e2e-tests/api-tests/src/test/java/org/eclipse/edc/identityhub/tests/PresentationApiEndToEndTest.java @@ -79,6 +79,7 @@ import static org.eclipse.edc.identityhub.verifiablecredentials.testfixtures.VerifiableCredentialTestUtil.generateEcKey; import static org.eclipse.edc.util.io.Ports.getFreePort; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.startsWith; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -220,6 +221,29 @@ void query_tokenVerificationFails_shouldReturn401(IdentityHubEndToEndTestContext .body("[0].message", equalTo("ID token verification failed: Token verification failed")); } + @Test + void query_proofOfPossessionFails_shouldReturn401(IdentityHubEndToEndTestContext context) throws JOSEException { + + var accessToken = generateJwt(CONSUMER_DID, CONSUMER_DID, PROVIDER_DID, Map.of("scope", TEST_SCOPE), CONSUMER_KEY); + var token = generateJwt(PROVIDER_DID, PROVIDER_DID, "mismatching", Map.of("client_id", PROVIDER_DID, "token", accessToken), PROVIDER_KEY); + + + when(DID_PUBLIC_KEY_RESOLVER.resolveKey(eq("did:web:consumer#key1"))).thenReturn(Result.success(CONSUMER_KEY.toPublicKey())); + when(DID_PUBLIC_KEY_RESOLVER.resolveKey(eq("did:web:provider#key1"))).thenReturn(Result.success(PROVIDER_KEY.toPublicKey())); + + context.getResolutionEndpoint().baseRequest() + .contentType(JSON) + .header(AUTHORIZATION, token) + .body(VALID_QUERY_WITH_SCOPE) + .post("/v1/participants/%s/presentations/query".formatted(TEST_PARTICIPANT_CONTEXT_ID_ENCODED)) + .then() + .statusCode(401) + .log().ifValidationFails() + .body("[0].type", equalTo("AuthenticationFailed")) + .body("[0].message", startsWith("ID token verification failed: ID token [sub] claim is not equal to [token.sub] claim")); + + } + @Test void query_credentialQueryResolverFails_shouldReturn403(IdentityHubEndToEndTestContext context, CredentialStore store) throws JOSEException, JsonProcessingException { @@ -517,7 +541,7 @@ class Postgres extends Tests { private static final String DB_NAME = "runtime"; private static final Integer DB_PORT = getFreePort(); - + @RegisterExtension static IdentityHubCustomizableEndToEndExtension runtime; static PostgresSqlService server = new PostgresSqlService(DB_NAME, DB_PORT); From f6b8f54247bd95e5b49f12e099c3a8eefe86cff1 Mon Sep 17 00:00:00 2001 From: Enrico Risa Date: Wed, 17 Jul 2024 16:30:17 +0200 Subject: [PATCH 2/2] chore: deps file --- DEPENDENCIES | 1 - 1 file changed, 1 deletion(-) diff --git a/DEPENDENCIES b/DEPENDENCIES index 74f8a559d..1372a7567 100644 --- a/DEPENDENCIES +++ b/DEPENDENCIES @@ -215,7 +215,6 @@ maven/mavencentral/org.apache.velocity/velocity-engine-core/2.3, Apache-2.0, app maven/mavencentral/org.apache.velocity/velocity-engine-scripting/2.3, Apache-2.0, restricted, clearlydefined maven/mavencentral/org.apache.xbean/xbean-reflect/3.7, Apache-2.0, approved, clearlydefined maven/mavencentral/org.apiguardian/apiguardian-api/1.1.2, Apache-2.0, approved, clearlydefined -maven/mavencentral/org.assertj/assertj-core/3.26.0, Apache-2.0, approved, #14886 maven/mavencentral/org.assertj/assertj-core/3.26.3, Apache-2.0, approved, #14886 maven/mavencentral/org.awaitility/awaitility/4.2.1, Apache-2.0, approved, #14178 maven/mavencentral/org.bouncycastle/bcpkix-jdk18on/1.72, MIT, approved, #3789