From 3aab0ba3c22d054107593f22dcf201d25eb4e866 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 15 Jul 2025 11:53:26 +0200 Subject: [PATCH] credentials.totpSecret as array and update credentials scenario test (#186) Co-authored-by: Michael Hoennig Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/186 Reviewed-by: Marc Sandlus --- .../login-credentials-data-model.mermaid | 2 +- .../hs/accounts/HsCredentialsController.java | 8 ++- .../hs/accounts/HsCredentialsEntity.java | 6 +- .../accounts/HsCredentialsEntityPatcher.java | 5 +- .../accounts/credentials-schemas.yaml | 23 +++++--- .../950-accounts/9510-hs-accounts.sql | 2 +- .../9519-hs-accounts-test-data.sql | 6 +- .../HsCredentialsEntityPatcherUnitTest.java | 17 +++--- .../scenarios/BaseCredentialsUseCase.java | 38 +++++++++++++ .../accounts/scenarios/CreateCredentials.java | 17 +++--- .../scenarios/CredentialsScenarioTests.java | 37 ++++++++---- .../accounts/scenarios/UpdateCredentials.java | 56 +++++++++++++++++++ .../hsadminng/hs/scenarios/PathAssertion.java | 5 +- .../hsadminng/hs/scenarios/ScenarioTest.java | 15 ++++- .../hs/scenarios/TemplateResolver.java | 6 +- .../hsadminng/hs/scenarios/UseCase.java | 2 +- 16 files changed, 192 insertions(+), 53 deletions(-) create mode 100644 src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/BaseCredentialsUseCase.java create mode 100644 src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/UpdateCredentials.java diff --git a/doc/ideas/login-credentials-data-model.mermaid b/doc/ideas/login-credentials-data-model.mermaid index 8f69450c..9bcf775a 100644 --- a/doc/ideas/login-credentials-data-model.mermaid +++ b/doc/ideas/login-credentials-data-model.mermaid @@ -9,7 +9,7 @@ classDiagram class Credentials{ +totpSecret: text - +telephonePassword: text + +phonePassword: text +emailAdress: text +smsNumber: text -active: bool [r/w] diff --git a/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsController.java b/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsController.java index fb597509..7ad6b681 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsController.java @@ -8,6 +8,7 @@ import java.util.function.BiConsumer; import io.micrometer.core.annotation.Timed; import io.swagger.v3.oas.annotations.security.SecurityRequirement; +import net.hostsharing.hsadminng.accounts.generated.api.v1.model.ContextResource; import net.hostsharing.hsadminng.config.MessageTranslator; import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.accounts.generated.api.v1.api.CredentialsApi; @@ -67,12 +68,12 @@ public class HsCredentialsController implements CredentialsApi { final UUID credentialsUuid) { context.assumeRoles(assumedRoles); - final var credentials = credentialsRepo.findByUuid(credentialsUuid); - if (credentials.isEmpty()) { + final var credentialsEntity = credentialsRepo.findByUuid(credentialsUuid); + if (credentialsEntity.isEmpty()) { return ResponseEntity.notFound().build(); } final var result = mapper.map( - credentials.get(), CredentialsResource.class, ENTITY_TO_RESOURCE_POSTMAPPER); + credentialsEntity.get(), CredentialsResource.class, ENTITY_TO_RESOURCE_POSTMAPPER); return ResponseEntity.ok(result); } @@ -192,6 +193,7 @@ public class HsCredentialsController implements CredentialsApi { mapper.map(person, HsOfficePersonResource.class) ) ); + resource.setContexts(mapper.mapList(entity.getLoginContexts().stream().toList(), ContextResource.class)); }; final BiConsumer RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> { diff --git a/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntity.java index 40215e90..178990d2 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntity.java @@ -11,6 +11,7 @@ import net.hostsharing.hsadminng.repr.Stringifyable; import java.time.LocalDateTime; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.UUID; @@ -30,7 +31,7 @@ public class HsCredentialsEntity implements BaseEntity, Str protected static Stringify stringify = stringify(HsCredentialsEntity.class, "credentials") .withProp(HsCredentialsEntity::isActive) .withProp(HsCredentialsEntity::getEmailAddress) - .withProp(HsCredentialsEntity::getTotpSecret) + .withProp(HsCredentialsEntity::getTotpSecrets) .withProp(HsCredentialsEntity::getPhonePassword) .withProp(HsCredentialsEntity::getSmsNumber) .quotedValues(false); @@ -66,7 +67,7 @@ public class HsCredentialsEntity implements BaseEntity, Str private String onboardingToken; @Column - private String totpSecret; + private List totpSecrets; @Column private String phonePassword; @@ -106,4 +107,5 @@ public class HsCredentialsEntity implements BaseEntity, Str public String toString() { return stringify.apply(this); } + } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntityPatcher.java b/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntityPatcher.java index 765e684e..3e424e47 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntityPatcher.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntityPatcher.java @@ -4,6 +4,7 @@ import net.hostsharing.hsadminng.accounts.generated.api.v1.model.CredentialsPatc import net.hostsharing.hsadminng.mapper.EntityPatcher; import net.hostsharing.hsadminng.mapper.OptionalFromJson; +import java.util.Optional; public class HsCredentialsEntityPatcher implements EntityPatcher { @@ -22,8 +23,8 @@ public class HsCredentialsEntityPatcher implements EntityPatcher INITIAL_TOTP_SECRETS = List.of("initial_2fa"); private static final String INITIAL_SMS_NUMBER = "initial_sms"; private static final String INITIAL_PHONE_PASSWORD = "initial_phone_pw"; private static final Boolean PATCHED_ACTIVE = false; private static final String PATCHED_EMAIL_ADDRESS = "patched@example.com"; - private static final String PATCHED_TOTP_SECRET = "patched_2fa"; + private static final List PATCHED_TOTP_SECRETS = List.of("patched_2fa"); private static final String PATCHED_SMS_NUMBER = "patched_sms"; private static final String PATCHED_PHONE_PASSWORD = "patched_phone_pw"; @@ -102,7 +102,7 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< entity.setUuid(INITIAL_CREDENTIALS_UUID); entity.setActive(INITIAL_ACTIVE); entity.setEmailAddress(INITIAL_EMAIL_ADDRESS); - entity.setTotpSecret(INITIAL_TOTP_SECRET); + entity.setTotpSecrets(INITIAL_TOTP_SECRETS); entity.setSmsNumber(INITIAL_SMS_NUMBER); entity.setPhonePassword(INITIAL_PHONE_PASSWORD); // Ensure loginContexts is a mutable set for the patcher @@ -137,12 +137,13 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< PATCHED_EMAIL_ADDRESS, HsCredentialsEntity::setEmailAddress, PATCHED_EMAIL_ADDRESS), - new JsonNullableProperty<>( + new SimpleProperty<>( "totpSecret", - CredentialsPatchResource::setTotpSecret, - PATCHED_TOTP_SECRET, - HsCredentialsEntity::setTotpSecret, - PATCHED_TOTP_SECRET), + CredentialsPatchResource::setTotpSecrets, + PATCHED_TOTP_SECRETS, + HsCredentialsEntity::setTotpSecrets, + PATCHED_TOTP_SECRETS) + .notNullable(), new JsonNullableProperty<>( "smsNumber", CredentialsPatchResource::setSmsNumber, diff --git a/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/BaseCredentialsUseCase.java b/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/BaseCredentialsUseCase.java new file mode 100644 index 00000000..022af18b --- /dev/null +++ b/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/BaseCredentialsUseCase.java @@ -0,0 +1,38 @@ +package net.hostsharing.hsadminng.hs.accounts.scenarios; + +import lombok.SneakyThrows; +import net.hostsharing.hsadminng.accounts.generated.api.v1.model.ContextResource; +import net.hostsharing.hsadminng.hs.scenarios.ScenarioTest; +import net.hostsharing.hsadminng.hs.scenarios.UseCase; +import org.apache.commons.lang3.tuple.Pair; + +import java.util.Arrays; + +import static io.restassured.http.ContentType.JSON; +import static org.springframework.http.HttpStatus.OK; + +public abstract class BaseCredentialsUseCase> extends UseCase { + + public BaseCredentialsUseCase(final ScenarioTest testSuite) { + super(testSuite); + } + + @SneakyThrows + protected ContextResource[] fetchContextResourcesByDescriptorPairs(final String descriptPairsVarName) { + final var requestedContexts = ScenarioTest.getTypedVariable("contexts", Pair[].class); + final var existingContextsJson = withTitle("Fetch Available Account Contexts", () -> + httpGet("/api/hs/accounts/contexts").expecting(OK).expecting(JSON) + ).getResponse().body(); + final var existingContexts = objectMapper.readValue(existingContextsJson, ContextResource[].class); + return Arrays.stream(requestedContexts) + .map(pair -> Arrays.stream(existingContexts) + .filter(context -> context.getType().equals(pair.getLeft()) + && context.getQualifier().equals(pair.getRight())) + .findFirst() + .orElseThrow(() -> new IllegalStateException( + "No matching context found for type=" + pair.getLeft() + + " and qualifier=" + pair.getRight())) + ) + .toArray(ContextResource[]::new); + } +} diff --git a/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/CreateCredentials.java b/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/CreateCredentials.java index d63be776..0e7191c8 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/CreateCredentials.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/CreateCredentials.java @@ -8,7 +8,7 @@ import org.springframework.http.HttpStatus; import static io.restassured.http.ContentType.JSON; import static org.springframework.http.HttpStatus.OK; -public class CreateCredentials extends UseCase { +public class CreateCredentials extends BaseCredentialsUseCase { public CreateCredentials(final ScenarioTest testSuite) { super(testSuite); @@ -26,9 +26,8 @@ public class CreateCredentials extends UseCase { "In real situations we have more precise measures to find the related person." ); - - obtain("CredentialsContexts", () -> - httpGet("/api/hs/accounts/contexts").expecting(OK).expecting(JSON) + given("resolvedContexts", + fetchContextResourcesByDescriptorPairs("contexts") ); return obtain("newCredentials", () -> @@ -37,12 +36,14 @@ public class CreateCredentials extends UseCase { "person.uuid": ${Person: %{personGivenName} %{personFamilyName}}, "nickname": ${nickname}, "active": %{active}, + "totpSecrets": @{totpSecrets}, "emailAddress": ${emailAddress}, - "telephonePassword": ${telephonePassword}, + "phonePassword": ${phonePassword}, "smsNumber": ${smsNumber}, + "onboardingToken": ${onboardingToken}, "globalUid": %{globalUid}, "globalGid": %{globalGid}, - "contexts": @{contexts} + "contexts": @{resolvedContexts} } """)) .expecting(HttpStatus.CREATED).expecting(ContentType.JSON) @@ -57,7 +58,9 @@ public class CreateCredentials extends UseCase { .expecting(OK).expecting(JSON), path("uuid").contains("%{newCredentials}"), path("nickname").contains("%{nickname}"), - path("person.uuid").contains("%{Person: %{personGivenName} %{personFamilyName}}") + path("person.uuid").contains("%{Person: %{personGivenName} %{personFamilyName}}"), + path("totpSecrets").contains("@{totpSecrets}"), + path("onboardingToken").contains("%{onboardingToken}") ); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/CredentialsScenarioTests.java b/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/CredentialsScenarioTests.java index 6705e5eb..7711e195 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/CredentialsScenarioTests.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/CredentialsScenarioTests.java @@ -4,10 +4,12 @@ import lombok.SneakyThrows; import net.hostsharing.hsadminng.HsadminNgApplication; import net.hostsharing.hsadminng.config.DisableSecurityConfig; import net.hostsharing.hsadminng.hs.scenarios.Produces; +import net.hostsharing.hsadminng.hs.scenarios.Requires; import net.hostsharing.hsadminng.hs.scenarios.ScenarioTest; import net.hostsharing.hsadminng.mapper.Array; import net.hostsharing.hsadminng.rbac.test.JpaAttempt; import net.hostsharing.hsadminng.test.IgnoreOnFailureExtension; +import org.apache.commons.lang3.tuple.Pair; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.ClassOrderer; import org.junit.jupiter.api.MethodOrderer; @@ -22,8 +24,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.context.ActiveProfiles; -import java.util.Map; - @Tag("scenarioTest") @SpringBootTest( webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, @@ -45,6 +45,7 @@ class CredentialsScenarioTests extends ScenarioTest { protected void beforeScenario(final TestInfo testInfo) { super.beforeScenario(testInfo); } + @Nested @Order(10) @TestMethodOrder(MethodOrderer.OrderAnnotation.class) @@ -62,22 +63,38 @@ class CredentialsScenarioTests extends ScenarioTest { .given("nickname", "firby-susan") // initial credentials .given("active", true) + .given("totpSecrets", Array.of("initialSecret")) .given("emailAddress", "susan.firby@example.com") - .given("telephonePassword", "securePass123") + .given("phonePassword", "securePass123") .given("smsNumber", "+49123456789") .given("globalUid", 21011) .given("globalGid", 21011) .given("contexts", Array.of( - Map.ofEntries( - // a hardcoded context from test-data - // TODO.impl: the uuid should be determined within CreateCredentials just by (HSDAMIN,prod) - Map.entry("uuid", "11111111-1111-1111-1111-111111111111"), - Map.entry("type", "HSADMIN"), - Map.entry("qualifier", "prod") - ) + Pair.of("HSADMIN", "prod") )) + .given("onboardingToken", "fake-unboarding-token") .doRun() .keep(); } + + @Test + @Order(1020) + @Requires("Credentials@hsadmin: firby-susan") + void shouldUpdateCredentials() { + new UpdateCredentials(scenarioTest) + // the credentials to update + .given("credentialsUuid", "%{Credentials@hsadmin: firby-susan}") + // updated credentials + .given("active", false) + .given("totpSecrets", Array.of("initialSecret", "additionalSecret")) + .given("emailAddress", "susan.firby@example.org") + .given("phonePassword", "securePass987") + .given("smsNumber", "+49987654321") + .given("contexts", Array.of( + Pair.of("HSADMIN", "prod"), + Pair.of("SSH", "internal") + )) + .doRun(); + } } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/UpdateCredentials.java b/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/UpdateCredentials.java new file mode 100644 index 00000000..4714bc1e --- /dev/null +++ b/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/UpdateCredentials.java @@ -0,0 +1,56 @@ +package net.hostsharing.hsadminng.hs.accounts.scenarios; + +import io.restassured.http.ContentType; +import net.hostsharing.hsadminng.hs.scenarios.ScenarioTest; +import net.hostsharing.hsadminng.hs.scenarios.UseCase; +import org.springframework.http.HttpStatus; + +import static io.restassured.http.ContentType.JSON; +import static org.springframework.http.HttpStatus.OK; + +public class UpdateCredentials extends BaseCredentialsUseCase { + + public UpdateCredentials(final ScenarioTest testSuite) { + super(testSuite); + + introduction("A set of credentials contains the login data for an RBAC subject."); + } + + @Override + protected HttpResponse run() { + + given("resolvedContexts", + fetchContextResourcesByDescriptorPairs("contexts") + ); + + withTitle("Patch the Changes to the existing Credentials", () -> + httpPatch("/api/hs/accounts/credentials/%{credentialsUuid}", usingJsonBody(""" + { + "active": %{active}, + "totpSecrets": @{totpSecrets}, + "emailAddress": ${emailAddress}, + "phonePassword": ${phonePassword}, + "smsNumber": ${smsNumber}, + "contexts": @{resolvedContexts} + } + """)) + .reportWithResponse().expecting(HttpStatus.OK).expecting(ContentType.JSON) + .extractValue("nickname", "nickname") + .extractValue("totpSecrets", "totpSecrets") + ); + + return null; + } + + @Override + protected void verify(final UseCase.HttpResponse response) { + verify( + "Verify the Patched Credentials", + () -> httpGet("/api/hs/accounts/credentials/%{credentialsUuid}") + .expecting(OK).expecting(JSON), + path("uuid").contains("%{newCredentials}"), + path("nickname").contains("%{nickname}"), + path("totpSecrets").contains("%{totpSecrets}") + ); + } +} diff --git a/src/test/java/net/hostsharing/hsadminng/hs/scenarios/PathAssertion.java b/src/test/java/net/hostsharing/hsadminng/hs/scenarios/PathAssertion.java index 04ec0fe9..64c5f2c6 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/scenarios/PathAssertion.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/scenarios/PathAssertion.java @@ -18,12 +18,13 @@ public class PathAssertion { @SuppressWarnings({ "unchecked", "rawtypes" }) public Consumer contains(final String resolvableValue) { + final var resolvedValue = ScenarioTest.resolve(resolvableValue, DROP_COMMENTS); return response -> { try { - response.path(path).isEqualTo(ScenarioTest.resolve(resolvableValue, DROP_COMMENTS)); + response.path(path).isEqualTo(resolvedValue); } catch (final AssertionError e) { // without this, the error message is often lacking important context - fail(e.getMessage() + " in `path(\"" + path + "\").contains(\"" + resolvableValue + "\")`" ); + fail(e.getMessage() + " in `path(\"" + path + "\").contains(\"" + resolvedValue + "\")`" ); } }; } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/scenarios/ScenarioTest.java b/src/test/java/net/hostsharing/hsadminng/hs/scenarios/ScenarioTest.java index 99d2f42e..01ab680e 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/scenarios/ScenarioTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/scenarios/ScenarioTest.java @@ -66,13 +66,13 @@ public abstract class ScenarioTest extends ContextBasedTest { keepProducesAlias(currentTestMethod); }); testReport.createTestLogMarkdownFile(testInfo); - } catch (Exception exc) { + } catch (final Exception exc) { throw exc; } } @AfterEach - void afterScenario(final TestInfo testInfo) { // final TestInfo testInfo + void afterScenario(final TestInfo testInfo) { verifyProduceDeclaration(testInfo); properties.clear(); @@ -191,7 +191,7 @@ public abstract class ScenarioTest extends ContextBasedTest { properties.remove(propName); } - static Map knowVariables() { + public static Map knowVariables() { final var map = new LinkedHashMap(); map.putAll(ScenarioTest.aliases); map.putAll(ScenarioTest.properties); @@ -223,4 +223,13 @@ public abstract class ScenarioTest extends ContextBasedTest { return (T) resolvedValue; } + public static T getTypedVariable(final String varName, final Class expectedValueClass) { + final var value = knowVariables().get(varName); + if (value != null && !expectedValueClass.isAssignableFrom(value.getClass())) { + throw new IllegalArgumentException("variable '" + varName + "'" + + " expected to be of type " + expectedValueClass + " " + + " but got " + value.getClass()); + } + return (T) value; + } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/scenarios/TemplateResolver.java b/src/test/java/net/hostsharing/hsadminng/hs/scenarios/TemplateResolver.java index 65e4d70d..9bc52cc9 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/scenarios/TemplateResolver.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/scenarios/TemplateResolver.java @@ -1,5 +1,7 @@ package net.hostsharing.hsadminng.hs.scenarios; +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.SneakyThrows; import org.apache.commons.lang3.StringUtils; import java.net.URLEncoder; @@ -17,6 +19,7 @@ import static net.hostsharing.hsadminng.hs.scenarios.TemplateResolver.Resolver.D public class TemplateResolver { public static final String JSON_NULL_VALUE_TO_KEEP = "NULL"; + public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); public enum Resolver { DROP_COMMENTS, // deletes comments ('#{whatever}' -> '') @@ -230,6 +233,7 @@ public class TemplateResolver { }; } + @SneakyThrows private static String jsonObject(final Object value) { return switch (value) { case null -> "null"; @@ -237,7 +241,7 @@ public class TemplateResolver { .map(entry -> "\"" + entry.getKey() + "\": " + jsonQuoted(entry.getValue())) .collect(Collectors.joining(", ")) + "}"; case String string -> "{" + string.replace("\n", " ") + "}"; - default -> throw new IllegalArgumentException("can not format " + value.getClass() + " (" + value + ") as JSON object"); + default -> OBJECT_MAPPER.writeValueAsString(value); }; } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/scenarios/UseCase.java b/src/test/java/net/hostsharing/hsadminng/hs/scenarios/UseCase.java index e8cdd946..69d687a5 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/scenarios/UseCase.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/scenarios/UseCase.java @@ -47,7 +47,7 @@ public abstract class UseCase> { private static final HttpClient client = HttpClient.newHttpClient(); private static final int HTTP_TIMEOUT_SECONDS = 20; // FIXME: configurable in environment - private final ObjectMapper objectMapper = new ObjectMapper(); + protected final ObjectMapper objectMapper = new ObjectMapper(); protected final ScenarioTest testSuite; private final TestReport testReport;