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.

Sign up for GitHub

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

fix: set keypair to ACTIVE #286

Merged
merged 2 commits into from
Feb 29, 2024
Merged

fix: set keypair to ACTIVE #286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7285aad
fix: improve handling of KeyPairResource state
paullatzelsperger Feb 29, 2024
92caf3c
DEPENDENCIES
paullatzelsperger Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions DEPENDENCIES
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
maven/mavencentral/com.apicatalog/carbon-did/0.0.2, Apache-2.0, approved, #9239

Check warning on line 1 in DEPENDENCIES

View workflow job for this annotation

GitHub Actions / check / Dash-Verify-Licenses

Restricted Dependencies found 

Some dependencies are marked 'restricted' - please review them
maven/mavencentral/com.apicatalog/iron-verifiable-credentials/0.8.1, Apache-2.0, approved, #9234
maven/mavencentral/com.apicatalog/titanium-json-ld/1.0.0, Apache-2.0, approved, clearlydefined
maven/mavencentral/com.apicatalog/titanium-json-ld/1.3.1, Apache-2.0, approved, #8912
Expand Down Expand Up @@ -42,8 +42,11 @@
maven/mavencentral/com.fasterxml.uuid/java-uuid-generator/4.1.0, Apache-2.0, approved, clearlydefined
maven/mavencentral/com.github.cliftonlabs/json-simple/3.0.2, Apache-2.0, approved, clearlydefined
maven/mavencentral/com.github.docker-java/docker-java-api/3.3.4, Apache-2.0, approved, #10346
maven/mavencentral/com.github.docker-java/docker-java-api/3.3.5, Apache-2.0, approved, #10346
maven/mavencentral/com.github.docker-java/docker-java-transport-zerodep/3.3.4, Apache-2.0 AND (Apache-2.0 AND BSD-3-Clause), approved, #7946
maven/mavencentral/com.github.docker-java/docker-java-transport-zerodep/3.3.5, Apache-2.0 AND (Apache-2.0 AND BSD-3-Clause), approved, #7946
maven/mavencentral/com.github.docker-java/docker-java-transport/3.3.4, Apache-2.0, approved, #7942
maven/mavencentral/com.github.docker-java/docker-java-transport/3.3.5, Apache-2.0, approved, #7942
maven/mavencentral/com.github.java-json-tools/btf/1.3, Apache-2.0 OR LGPL-3.0-or-later, approved, #2721
maven/mavencentral/com.github.java-json-tools/jackson-coreutils-equivalence/1.0, LGPL-3.0 OR Apache-2.0, approved, clearlydefined
maven/mavencentral/com.github.java-json-tools/jackson-coreutils/2.0, Apache-2.0 OR LGPL-3.0-or-later, approved, #2719
Expand Down Expand Up @@ -78,7 +81,7 @@
maven/mavencentral/com.networknt/json-schema-validator/1.0.76, Apache-2.0, approved, CQ22638
maven/mavencentral/com.nimbusds/nimbus-jose-jwt/9.28, Apache-2.0, approved, clearlydefined
maven/mavencentral/com.nimbusds/nimbus-jose-jwt/9.37.3, Apache-2.0, approved, #11701
maven/mavencentral/com.puppycrawl.tools/checkstyle/10.13.0, , restricted, clearlydefined
maven/mavencentral/com.puppycrawl.tools/checkstyle/10.14.0, , restricted, clearlydefined
maven/mavencentral/com.samskivert/jmustache/1.15, BSD-2-Clause, approved, clearlydefined
maven/mavencentral/com.squareup.okhttp3/okhttp-dnsoverhttps/4.12.0, Apache-2.0, approved, #11159
maven/mavencentral/com.squareup.okhttp3/okhttp/4.12.0, Apache-2.0, approved, #11156
Expand Down Expand Up @@ -362,7 +365,7 @@
maven/mavencentral/org.ow2.asm/asm-tree/9.6, BSD-3-Clause, approved, #10773
maven/mavencentral/org.ow2.asm/asm/9.1, BSD-3-Clause, approved, CQ23029
maven/mavencentral/org.ow2.asm/asm/9.6, BSD-3-Clause, approved, #10776
maven/mavencentral/org.postgresql/postgresql/42.7.1, BSD-2-Clause AND Apache-2.0, approved, #11681
maven/mavencentral/org.postgresql/postgresql/42.7.2, BSD-2-Clause AND Apache-2.0, approved, #11681
maven/mavencentral/org.reflections/reflections/0.10.2, Apache-2.0 AND WTFPL, approved, clearlydefined
maven/mavencentral/org.rnorth.duct-tape/duct-tape/1.0.8, MIT, approved, clearlydefined
maven/mavencentral/org.slf4j/slf4j-api/1.7.22, MIT, approved, CQ11943
Expand All @@ -373,13 +376,13 @@
maven/mavencentral/org.slf4j/slf4j-api/1.7.36, MIT, approved, CQ13368
maven/mavencentral/org.slf4j/slf4j-api/2.0.6, MIT, approved, #5915
maven/mavencentral/org.slf4j/slf4j-api/2.0.9, MIT, approved, #5915
maven/mavencentral/org.testcontainers/database-commons/1.19.5, Apache-2.0, approved, #10345
maven/mavencentral/org.testcontainers/jdbc/1.19.5, Apache-2.0, approved, #10348
maven/mavencentral/org.testcontainers/database-commons/1.19.6, Apache-2.0, approved, #10345
maven/mavencentral/org.testcontainers/jdbc/1.19.6, Apache-2.0, approved, #10348
maven/mavencentral/org.testcontainers/junit-jupiter/1.19.3, MIT, approved, #10344
maven/mavencentral/org.testcontainers/junit-jupiter/1.19.5, MIT, approved, #10344
maven/mavencentral/org.testcontainers/postgresql/1.19.5, MIT, approved, #10350
maven/mavencentral/org.testcontainers/junit-jupiter/1.19.6, MIT, approved, #10344
maven/mavencentral/org.testcontainers/postgresql/1.19.6, MIT, approved, #10350
maven/mavencentral/org.testcontainers/testcontainers/1.19.3, Apache-2.0 AND MIT, approved, #10347
maven/mavencentral/org.testcontainers/testcontainers/1.19.5, Apache-2.0 AND MIT, approved, #10347
maven/mavencentral/org.testcontainers/testcontainers/1.19.6, Apache-2.0 AND MIT, approved, #10347
maven/mavencentral/org.xmlresolver/xmlresolver/5.2.2, Apache-2.0, approved, clearlydefined
maven/mavencentral/org.xmlunit/xmlunit-core/2.9.1, Apache-2.0, approved, #6272
maven/mavencentral/org.xmlunit/xmlunit-placeholders/2.9.1, Apache-2.0, approved, clearlydefined
Expand Down
34 changes: 33 additions & 1 deletion ...y-hub-keypairs/src/main/java/org/eclipse/edc/identityhub/keypairs/KeyPairServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@

import java.time.Instant;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

Expand All @@ -63,10 +65,23 @@ public ServiceResult<Void> addKeyPair(String participantId, KeyDescriptor keyDes
return ServiceResult.badRequest(key.getFailureDetail());
}

// check if the new key is not active, and no other active key exists
if (!keyDescriptor.isActive()) {
var hasActiveKeys = keyPairResourceStore.query(QuerySpec.Builder.newInstance().filter(new Criterion("participantId", "=", participantId)).build())
.orElse(failure -> Collections.emptySet())
.stream().filter(kpr -> kpr.getState() == KeyPairState.ACTIVE.code())
.findAny()
.isEmpty();

if (!hasActiveKeys) {
monitor.warning("Participant '%s' has no active key pairs, and adding an inactive one will prevent the participant from becoming operational.");
}
}

var newResource = KeyPairResource.Builder.newInstance()
.id(keyDescriptor.getKeyId())
.keyId(keyDescriptor.getKeyId())
.state(KeyPairState.CREATED)
.state(keyDescriptor.isActive() ? KeyPairState.ACTIVE : KeyPairState.CREATED)
.isDefaultPair(makeDefault)
.privateKeyAlias(keyDescriptor.getPrivateKeyAlias())
.serializedPublicKey(key.getContent())
Expand Down Expand Up @@ -132,6 +147,23 @@ public ServiceResult<Collection<KeyPairResource>> query(QuerySpec querySpec) {
return ServiceResult.from(keyPairResourceStore.query(querySpec));
}

@Override
public ServiceResult<Void> activate(String keyPairResourceId) {
var oldKey = findById(keyPairResourceId);
if (oldKey == null) {
return ServiceResult.notFound("A KeyPairResource with ID '%s' does not exist.".formatted(keyPairResourceId));
}

var allowedStates = List.of(KeyPairState.ACTIVE.code(), KeyPairState.CREATED.code());
if (!allowedStates.contains(oldKey.getState())) {
return ServiceResult.badRequest("The key pair resource is expected to be in %s, but was %s".formatted(allowedStates, oldKey.getState()));
}

oldKey.activate();

return ServiceResult.from(keyPairResourceStore.update(oldKey));
}

@Override
public <E extends Event> void on(EventEnvelope<E> eventEnvelope) {
var payload = eventEnvelope.getPayload();
Expand Down
75 changes: 72 additions & 3 deletions ...b-keypairs/src/test/java/org/eclipse/edc/identityhub/keypairs/KeyPairServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.junit.jupiter.params.provider.ValueSource;

import java.time.Duration;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.UUID;
Expand All @@ -41,6 +42,7 @@
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
Expand All @@ -55,7 +57,7 @@ class KeyPairServiceImplTest {


@ParameterizedTest(name = "make default: {0}")
@ValueSource(booleans = {true, false})
@ValueSource(booleans = { true, false })
void addKeyPair_publicKeyGiven(boolean makeDefault) {

when(keyPairResourceStore.create(any())).thenReturn(success());
Expand All @@ -69,7 +71,7 @@ void addKeyPair_publicKeyGiven(boolean makeDefault) {
}

@ParameterizedTest(name = "make default: {0}")
@ValueSource(booleans = {true, false})
@ValueSource(booleans = { true, false })
void addKeyPair_shouldGenerate_storesInVault(boolean makeDefault) {
when(keyPairResourceStore.create(any())).thenReturn(success());

Expand All @@ -81,7 +83,34 @@ void addKeyPair_shouldGenerate_storesInVault(boolean makeDefault) {
assertThat(keyPairService.addKeyPair("some-participant", key, makeDefault)).isSucceeded();

verify(vault).storeSecret(eq(key.getPrivateKeyAlias()), anyString());
verify(keyPairResourceStore).create(argThat(kpr -> kpr.isDefaultPair() == makeDefault && kpr.getParticipantId().equals("some-participant")));
verify(keyPairResourceStore).create(argThat(kpr -> kpr.isDefaultPair() == makeDefault &&
kpr.getParticipantId().equals("some-participant") &&
kpr.getState() == KeyPairState.ACTIVE.code()));
verify(observableMock).invokeForEach(any());
verifyNoMoreInteractions(keyPairResourceStore, vault, observableMock);
}

@ParameterizedTest(name = "make active: {0}")
@ValueSource(booleans = { true, false })
void addKeyPair_assertActiveState(boolean isActive) {
when(keyPairResourceStore.query(any())).thenReturn(success(Collections.emptySet()));
when(keyPairResourceStore.create(any())).thenReturn(success());

var key = createKey().publicKeyJwk(null).publicKeyPem(null)
.active(isActive)
.keyGeneratorParams(Map.of(
"algorithm", "EdDSA",
"curve", "Ed25519"
)).build();

assertThat(keyPairService.addKeyPair("some-participant", key, true)).isSucceeded();

verify(vault).storeSecret(eq(key.getPrivateKeyAlias()), anyString());
//expect the query for other active keys at least once, if the new key is inactive
verify(keyPairResourceStore, isActive ? never() : times(1)).query(any());
verify(keyPairResourceStore).create(argThat(kpr -> kpr.isDefaultPair() &&
kpr.getParticipantId().equals("some-participant") &&
kpr.getState() == (isActive ? KeyPairState.ACTIVE.code() : KeyPairState.CREATED.code())));
verify(observableMock).invokeForEach(any());
verifyNoMoreInteractions(keyPairResourceStore, vault, observableMock);
}
Expand Down Expand Up @@ -268,6 +297,46 @@ void revokeKey_notfound() {
verifyNoMoreInteractions(keyPairResourceStore, vault, observableMock);
}

@ParameterizedTest(name = "Valid state = {0}")
// cannot use enum literals and the .code() method -> needs to be compile constant
@ValueSource(ints = { 100, 200 })
void activate(int validState) {
var oldId = "old-id";
var oldKey = createKeyPairResource().id(oldId).state(validState).build();

when(keyPairResourceStore.query(any())).thenReturn(success(List.of(oldKey)));
when(keyPairResourceStore.update(any())).thenReturn(success());

assertThat(keyPairService.activate(oldId)).isSucceeded();
}

@ParameterizedTest(name = "Valid state = {0}")
// cannot use enum literals and the .code() method -> needs to be compile constant
@ValueSource(ints = { 0, 30, 400, -10 })
void activate_invalidState(int validState) {
var oldId = "old-id";
var oldKey = createKeyPairResource().id(oldId).state(validState).build();

when(keyPairResourceStore.query(any())).thenReturn(success(List.of(oldKey)));
when(keyPairResourceStore.update(any())).thenReturn(success());

assertThat(keyPairService.activate(oldId))
.isFailed()
.detail()
.isEqualTo("The key pair resource is expected to be in [200, 100], but was %s".formatted(validState));
}

@Test
void activate_notExists() {

when(keyPairResourceStore.query(any())).thenReturn(success(List.of()));

assertThat(keyPairService.activate("notexists"))
.isFailed()
.detail()
.isEqualTo("A KeyPairResource with ID 'notexists' does not exist.");
}

private KeyPairResource.Builder createKeyPairResource() {
return KeyPairResource.Builder.newInstance()
.id(UUID.randomUUID().toString())
Expand Down
73 changes: 73 additions & 0 deletions ...tests/src/test/java/org/eclipse/edc/identityhub/tests/KeyPairResourceApiEndToEndTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.eclipse.edc.identityhub.spi.events.keypair.KeyPairAdded;
import org.eclipse.edc.identityhub.spi.events.keypair.KeyPairRotated;
import org.eclipse.edc.identityhub.spi.model.KeyPairResource;
import org.eclipse.edc.identityhub.spi.model.KeyPairState;
import org.eclipse.edc.identityhub.spi.model.participant.KeyDescriptor;
import org.eclipse.edc.identityhub.spi.model.participant.ParticipantContext;
import org.eclipse.edc.junit.annotations.EndToEndTest;
Expand Down Expand Up @@ -413,6 +414,78 @@ void getAll_notAuthorized() {
.statusCode(403);
}

@Test
void activate() {
var superUserKey = createSuperUser();
var user1 = "user1";
var token = createParticipant(user1);
var keyId = createKeyPair(user1);

assertThat(Arrays.asList(token, superUserKey))
.allSatisfy(t -> {
RUNTIME_CONFIGURATION.getManagementEndpoint().baseRequest()
.contentType(JSON)
.header(new Header("x-api-key", t))
.post("/v1/participants/%s/keypairs/%s/activate".formatted(toBase64(user1), keyId))
.then()
.log().ifValidationFails()
.statusCode(204)
.body(notNullValue());

assertThat(getDidForParticipant(user1))
.hasSize(1)
.allSatisfy(dd -> assertThat(dd.getVerificationMethod()).noneMatch(vm -> vm.getId().equals(keyId)));
});
}

@Test
void activate_notAuthorized() {
var user1 = "user1";
createParticipant(user1);
var keyId = createKeyPair(user1);
var attackerToken = createParticipant("attacker");

RUNTIME_CONFIGURATION.getManagementEndpoint().baseRequest()
.contentType(JSON)
.header(new Header("x-api-key", attackerToken))
.post("/v1/participants/%s/keypairs/%s/activate".formatted(toBase64(user1), keyId))
.then()
.log().ifValidationFails()
.statusCode(403)
.body(notNullValue());

assertThat(getKeyPairsForParticipant(user1))
.hasSize(2)
.allMatch(keyPairResource -> keyPairResource.getState() == KeyPairState.ACTIVE.code());
}

@Test
void activate_illegalState() {
var user1 = "user1";
var token = createParticipant(user1);
var keyId = createKeyPair(user1);

// first revoke the key, which puts it in the REVOKED state
RUNTIME_CONFIGURATION.getManagementEndpoint().baseRequest()
.contentType(JSON)
.header(new Header("x-api-key", token))
.post("/v1/participants/%s/keypairs/%s/revoke".formatted(toBase64(user1), keyId))
.then()
.log().ifValidationFails()
.statusCode(204)
.body(notNullValue());

// now attempt to activate
RUNTIME_CONFIGURATION.getManagementEndpoint().baseRequest()
.contentType(JSON)
.header(new Header("x-api-key", token))
.post("/v1/participants/%s/keypairs/%s/activate".formatted(toBase64(user1), keyId))
.then()
.log().ifValidationFails()
.statusCode(400)
.body(notNullValue());
}

private String createKeyPair(String participantId) {

var descriptor = createKeyDescriptor(participantId).build();
Expand Down
24 changes: 14 additions & 10 deletions .../api-tests/src/test/java/org/eclipse/edc/identityhub/tests/ManagementApiEndToEndTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.eclipse.edc.spi.query.Criterion;
import org.eclipse.edc.spi.query.QuerySpec;
import org.eclipse.edc.spi.security.Vault;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.extension.RegisterExtension;

import java.util.Collection;
Expand All @@ -50,23 +51,26 @@ public abstract class ManagementApiEndToEndTest {
@RegisterExtension
protected static final EdcRuntimeExtension RUNTIME = new EdcRuntimeExtension(":launcher", "identity-hub", RUNTIME_CONFIGURATION.controlPlaneConfiguration());

protected static ParticipantManifest createNewParticipant() {
protected static ParticipantManifest.Builder createNewParticipant() {
var manifest = ParticipantManifest.Builder.newInstance()
.participantId("another-participant")
.active(false)
.did("did:web:another:participant")
.serviceEndpoint(new Service("test-service", "test-service-type", "https://test.com"))
.key(KeyDescriptor.Builder.newInstance()
.privateKeyAlias("another-alias")
.keyGeneratorParams(Map.of("algorithm", "EdDSA", "curve", "Ed25519"))
.keyId("another-keyid")
.build())
.build();
.key(createKeyDescriptor().build());
return manifest;
}

@NotNull
public static KeyDescriptor.Builder createKeyDescriptor() {
return KeyDescriptor.Builder.newInstance()
.privateKeyAlias("another-alias")
.keyGeneratorParams(Map.of("algorithm", "EdDSA", "curve", "Ed25519"))
.keyId("another-keyid");
}

protected String createSuperUser() {
return createParticipant("super-user", List.of(ServicePrincipal.ROLE_ADMIN));
return createParticipant(SUPER_USER, List.of(ServicePrincipal.ROLE_ADMIN));
}

protected String storeParticipant(ParticipantContext pc) {
Expand All @@ -91,9 +95,9 @@ protected <T> T getService(Class<T> type) {
return RUNTIME.getContext().getService(type);
}

protected Collection<KeyPairResource> getKeyPairsForParticipant(ParticipantManifest manifest) {
protected Collection<KeyPairResource> getKeyPairsForParticipant(String participantId) {
return getService(KeyPairResourceStore.class).query(QuerySpec.Builder.newInstance()
.filter(new Criterion("participantId", "=", manifest.getParticipantId()))
.filter(new Criterion("participantId", "=", participantId))
.build()).getContent();
}

Expand Down
Loading
Loading