From 4a115f7acefce5fb87dad15b18fc9fabbf194f35 Mon Sep 17 00:00:00 2001 From: Paul Latzelsperger Date: Mon, 30 Sep 2024 17:26:01 +0200 Subject: [PATCH 1/2] fix(dcp): prevent default identity extractor from being registered --- .../edc/iam/iatp/IatpIdentityExtension.java | 17 ++-- .../iatp/identity/IatpIdentityExtractor.java | 5 +- .../tests/transfer/IdentityExtractorTest.java | 83 +++++++++++++++++++ 3 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 edc-tests/edc-controlplane/iatp-tests/src/test/java/org/eclipse/tractusx/edc/tests/transfer/IdentityExtractorTest.java diff --git a/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtension.java b/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtension.java index 7f8213771..a9e73b712 100644 --- a/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtension.java +++ b/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtension.java @@ -19,9 +19,9 @@ package org.eclipse.tractusx.edc.iam.iatp; +import org.eclipse.edc.iam.identitytrust.spi.DcpParticipantAgentServiceExtension; import org.eclipse.edc.runtime.metamodel.annotation.Extension; -import org.eclipse.edc.runtime.metamodel.annotation.Inject; -import org.eclipse.edc.spi.agent.ParticipantAgentService; +import org.eclipse.edc.runtime.metamodel.annotation.Provider; import org.eclipse.edc.spi.system.ServiceExtension; import org.eclipse.edc.spi.system.ServiceExtensionContext; import org.eclipse.tractusx.edc.iam.iatp.identity.IatpIdentityExtractor; @@ -33,8 +33,7 @@ public class IatpIdentityExtension implements ServiceExtension { static final String NAME = "Tractusx IATP identity extension"; - @Inject - private ParticipantAgentService participantAgentService; + private final IatpIdentityExtractor iatpIdentityExtractor = new IatpIdentityExtractor(); @Override public String name() { @@ -43,7 +42,15 @@ public String name() { @Override public void initialize(ServiceExtensionContext context) { - participantAgentService.register(new IatpIdentityExtractor()); + } + + /** + * This provider method is mandatory, because it prevents the {@code DefaultDcpParticipantAgentServiceExtension} from being + * registered, which would cause a race condition in the identity extractors + */ + @Provider + public DcpParticipantAgentServiceExtension extractor() { + return iatpIdentityExtractor; } } diff --git a/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/identity/IatpIdentityExtractor.java b/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/identity/IatpIdentityExtractor.java index 59754f3c1..f3678fb90 100644 --- a/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/identity/IatpIdentityExtractor.java +++ b/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/identity/IatpIdentityExtractor.java @@ -19,6 +19,7 @@ package org.eclipse.tractusx.edc.iam.iatp.identity; +import org.eclipse.edc.iam.identitytrust.spi.DcpParticipantAgentServiceExtension; import org.eclipse.edc.iam.verifiablecredentials.spi.model.VerifiableCredential; import org.eclipse.edc.spi.EdcException; import org.eclipse.edc.spi.agent.ParticipantAgentServiceExtension; @@ -38,7 +39,7 @@ * Implementation of {@link ParticipantAgentServiceExtension} which extracts the identity of a participant * from the MembershipCredential */ -public class IatpIdentityExtractor implements ParticipantAgentServiceExtension { +public class IatpIdentityExtractor implements DcpParticipantAgentServiceExtension { private static final String VC_CLAIM = "vc"; private static final String IDENTITY_CREDENTIAL = "MembershipCredential"; @@ -56,7 +57,7 @@ public class IatpIdentityExtractor implements ParticipantAgentServiceExtension { .findFirst() .flatMap(this::getIdentifier) .map(identity -> Map.of(PARTICIPANT_IDENTITY, identity)) - .orElseThrow(() -> new EdcException("Failed to fetch %s property from %s credential".formatted(IDENTITY_PROPERTY, IDENTITY_CREDENTIAL))); + .orElseThrow(() -> new EdcException("Required credential type '%s' not present in ClaimToken, cannot extract property '%s'".formatted(IDENTITY_CREDENTIAL, IDENTITY_PROPERTY))); } diff --git a/edc-tests/edc-controlplane/iatp-tests/src/test/java/org/eclipse/tractusx/edc/tests/transfer/IdentityExtractorTest.java b/edc-tests/edc-controlplane/iatp-tests/src/test/java/org/eclipse/tractusx/edc/tests/transfer/IdentityExtractorTest.java new file mode 100644 index 000000000..40e565d92 --- /dev/null +++ b/edc-tests/edc-controlplane/iatp-tests/src/test/java/org/eclipse/tractusx/edc/tests/transfer/IdentityExtractorTest.java @@ -0,0 +1,83 @@ +/******************************************************************************** + * Copyright (c) 2024 Bayerische Motoren Werke Aktiengesellschaft (BMW AG) + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * 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. + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ + +package org.eclipse.tractusx.edc.tests.transfer; + +import org.eclipse.edc.iam.verifiablecredentials.spi.model.CredentialSubject; +import org.eclipse.edc.iam.verifiablecredentials.spi.model.Issuer; +import org.eclipse.edc.iam.verifiablecredentials.spi.model.VerifiableCredential; +import org.eclipse.edc.junit.annotations.EndToEndTest; +import org.eclipse.edc.junit.extensions.RuntimeExtension; +import org.eclipse.edc.spi.EdcException; +import org.eclipse.edc.spi.agent.ParticipantAgentService; +import org.eclipse.edc.spi.iam.ClaimToken; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import java.time.Instant; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.eclipse.tractusx.edc.tests.transfer.iatp.runtime.Runtimes.iatpRuntime; + +/** + * This test asserts that the ParticipantAgent's identity is determined by the "credentialSubject.holderIdentifier" property. + * Due to how the extractors are used and registered, this must be tested using a fully-fledged runtime. + */ +@EndToEndTest +public class IdentityExtractorTest implements IatpParticipants { + + @RegisterExtension + protected static final RuntimeExtension CONSUMER_RUNTIME = iatpRuntime(CONSUMER.getName(), CONSUMER.iatpConfiguration(PROVIDER), CONSUMER.getKeyPair()); + + @Test + void verifyCorrectParticipantAgentId(ParticipantAgentService participantAgentService) { + var claimtoken = ClaimToken.Builder.newInstance() + .claim("vc", List.of(createCredential().build())) + .build(); + var agent = participantAgentService.createFor(claimtoken); + + assertThat(agent.getIdentity()).isEqualTo("the-holder"); + } + + @Test + void verifyAgentId_whenNoMembershipCredential(ParticipantAgentService participantAgentService) { + var claimtoken = ClaimToken.Builder.newInstance() + .claim("vc", List.of(createCredential().types(List.of("VerifiableCredential")).build())) + .build(); + assertThatThrownBy(() -> participantAgentService.createFor(claimtoken)).isInstanceOf(EdcException.class) + .hasMessage("Required credential type 'MembershipCredential' not present in ClaimToken, cannot extract property 'holderIdentifier'"); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private VerifiableCredential.Builder createCredential() { + return VerifiableCredential.Builder.newInstance() + .types(List.of("VerifiableCredential", "MembershipCredential")) + .id("test-id") + .issuanceDate(Instant.now()) + .issuer(new Issuer("test-issuer", Map.of())) + .credentialSubject(CredentialSubject.Builder.newInstance() + .id("test-id") + .claim("holderIdentifier", "the-holder") + .build()); + } + +} From f5c8d7aab2ee55403e4a181dc723fa7d948b4b11 Mon Sep 17 00:00:00 2001 From: Paul Latzelsperger Date: Mon, 30 Sep 2024 18:36:29 +0200 Subject: [PATCH 2/2] fix unit tests --- .../edc/iam/iatp/IatpIdentityExtension.java | 4 ---- .../iam/iatp/IatpIdentityExtensionTest.java | 19 +++---------------- .../identity/IatpIdentityExtractorTest.java | 12 ++++++------ 3 files changed, 9 insertions(+), 26 deletions(-) diff --git a/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtension.java b/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtension.java index a9e73b712..f3cbad85f 100644 --- a/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtension.java +++ b/edc-extensions/dcp/tx-dcp/src/main/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtension.java @@ -23,7 +23,6 @@ import org.eclipse.edc.runtime.metamodel.annotation.Extension; import org.eclipse.edc.runtime.metamodel.annotation.Provider; import org.eclipse.edc.spi.system.ServiceExtension; -import org.eclipse.edc.spi.system.ServiceExtensionContext; import org.eclipse.tractusx.edc.iam.iatp.identity.IatpIdentityExtractor; import static org.eclipse.tractusx.edc.iam.iatp.IatpDefaultScopeExtension.NAME; @@ -40,9 +39,6 @@ public String name() { return NAME; } - @Override - public void initialize(ServiceExtensionContext context) { - } /** * This provider method is mandatory, because it prevents the {@code DefaultDcpParticipantAgentServiceExtension} from being diff --git a/edc-extensions/dcp/tx-dcp/src/test/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtensionTest.java b/edc-extensions/dcp/tx-dcp/src/test/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtensionTest.java index 71e45ab12..2d24692c4 100644 --- a/edc-extensions/dcp/tx-dcp/src/test/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtensionTest.java +++ b/edc-extensions/dcp/tx-dcp/src/test/java/org/eclipse/tractusx/edc/iam/iatp/IatpIdentityExtensionTest.java @@ -20,31 +20,18 @@ package org.eclipse.tractusx.edc.iam.iatp; import org.eclipse.edc.junit.extensions.DependencyInjectionExtension; -import org.eclipse.edc.spi.agent.ParticipantAgentService; -import org.eclipse.edc.spi.system.ServiceExtensionContext; import org.eclipse.tractusx.edc.iam.iatp.identity.IatpIdentityExtractor; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import static org.mockito.ArgumentMatchers.isA; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; +import static org.assertj.core.api.Assertions.assertThat; @ExtendWith(DependencyInjectionExtension.class) public class IatpIdentityExtensionTest { - private final ParticipantAgentService participantAgentService = mock(); - - @BeforeEach - void setup(ServiceExtensionContext context) { - context.registerService(ParticipantAgentService.class, participantAgentService); - } - @Test - void initialize(ServiceExtensionContext context, IatpIdentityExtension extension) { - extension.initialize(context); - verify(participantAgentService).register(isA(IatpIdentityExtractor.class)); + void initialize(IatpIdentityExtension extension) { + assertThat(extension.extractor()).isInstanceOf(IatpIdentityExtractor.class); } } diff --git a/edc-extensions/dcp/tx-dcp/src/test/java/org/eclipse/tractusx/edc/iam/iatp/identity/IatpIdentityExtractorTest.java b/edc-extensions/dcp/tx-dcp/src/test/java/org/eclipse/tractusx/edc/iam/iatp/identity/IatpIdentityExtractorTest.java index 5e2727dea..8965b7a35 100644 --- a/edc-extensions/dcp/tx-dcp/src/test/java/org/eclipse/tractusx/edc/iam/iatp/identity/IatpIdentityExtractorTest.java +++ b/edc-extensions/dcp/tx-dcp/src/test/java/org/eclipse/tractusx/edc/iam/iatp/identity/IatpIdentityExtractorTest.java @@ -62,21 +62,21 @@ void attributesFor(VerifiableCredential credential) { } @Test - void attributesFor_Fails_WhenCredentialNotFound() { + void attributesFor_fails_WhenCredentialNotFound() { assertThatThrownBy(() -> extractor.attributesFor(ClaimToken.Builder.newInstance().claim("vc", List.of(vc("FooCredential", Map.of("foo", "bar")))).build())) .isInstanceOf(EdcException.class) - .hasMessageContaining("Failed to fetch"); + .hasMessage("Required credential type 'MembershipCredential' not present in ClaimToken, cannot extract property 'holderIdentifier'"); } @Test - void attributesFor_Fails_whenNoVcClaims() { + void attributesFor_fails_whenNoVcClaims() { assertThatThrownBy(() -> extractor.attributesFor(ClaimToken.Builder.newInstance().build())) .isInstanceOf(EdcException.class) .hasMessageContaining("Failed to fetch credentials from the claim token: ClaimToken did not contain a 'vc' claim"); } @Test - void attributesFor_Fails_whenNullVcClaims() { + void attributesFor_fails_whenNullVcClaims() { assertThatThrownBy(() -> extractor.attributesFor(ClaimToken.Builder.newInstance().claim("vc", null).build())) .isInstanceOf(EdcException.class) @@ -84,14 +84,14 @@ void attributesFor_Fails_whenNullVcClaims() { } @Test - void attributesFor_Fails_WhenVcClaimIsNotList() { + void attributesFor_fails_WhenVcClaimIsNotList() { assertThatThrownBy(() -> extractor.attributesFor(ClaimToken.Builder.newInstance().claim("vc", "wrong").build())) .isInstanceOf(EdcException.class) .hasMessageContaining("Failed to fetch credentials from the claim token: ClaimToken contains a 'vc' claim, but the type is incorrect. Expected java.util.List, got java.lang.String."); } @Test - void attributesFor_Fails_WhenVcClaimsIsEmptyList() { + void attributesFor_fails_WhenVcClaimsIsEmptyList() { assertThatThrownBy(() -> extractor.attributesFor(ClaimToken.Builder.newInstance().claim("vc", List.of()).build())) .isInstanceOf(EdcException.class) .hasMessageContaining("Failed to fetch credentials from the claim token: ClaimToken contains a 'vc' claim but it did not contain any VerifiableCredentials.");