From 6de920dc38a2dc8ef68e974dc2c442bc83ae3bbb Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 10 Sep 2025 14:26:47 +0200 Subject: [PATCH] Revert "remove secrets from credentials (#198)" (#200) This reverts commit 27b4f59a97fb59d032e8ddeac39d95217fbfcc35. Co-authored-by: Michael Hoennig Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/200 Reviewed-by: Timotheus Pokorra --- .tc-environment | 4 -- .unset-environment | 5 +-- bin/jwt-curl | 6 +-- .../login-credentials-data-model.mermaid | 3 ++ .../hs/accounts/HsCredentialsController.java | 21 ++++++++++ .../hs/accounts/HsCredentialsEntity.java | 16 ++++++++ .../accounts/HsCredentialsEntityPatcher.java | 5 +++ .../api-definition/accounts/api-paths.yaml | 3 ++ .../accounts/credentials-schemas.yaml | 26 ++++++++++++ .../accounts/credentials-with-uuid-used.yaml | 23 +++++++++++ .../950-accounts/9510-hs-accounts.sql | 4 ++ .../9519-hs-accounts-test-data.sql | 8 ++-- ...HsCredentialsControllerAcceptanceTest.java | 41 ++++++++++++++++++- .../HsCredentialsEntityPatcherUnitTest.java | 19 +++++++++ .../accounts/scenarios/CreateCredentials.java | 7 +++- .../scenarios/CredentialsScenarioTests.java | 5 +++ .../accounts/scenarios/UpdateCredentials.java | 7 +++- ...ceBankAccountControllerAcceptanceTest.java | 2 +- 18 files changed, 186 insertions(+), 19 deletions(-) create mode 100644 src/main/resources/api-definition/accounts/credentials-with-uuid-used.yaml diff --git a/.tc-environment b/.tc-environment index 063792cd..665a2123 100644 --- a/.tc-environment +++ b/.tc-environment @@ -5,9 +5,5 @@ export HSADMINNG_POSTGRES_ADMIN_USERNAME=admin export HSADMINNG_SUPERUSER=import-superuser@hostsharing.net export HSADMINNG_OFFICE_DATA_SQL_FILE export HSADMINNG_JWT_TOKEN_URL=http://localhost:8080/fake-jwt/token -export HSADMINNG_JWT_CLIENT_ID=hsscript.ng -export HSADMINNG_JWT_CLIENT_SECRET= -export HSADMINNG_JWT_USERNAME=superuser-alex@hostsharing.net -export HSADMINNG_JWT_PASSWORD=password export LANG=en_US.UTF-8 diff --git a/.unset-environment b/.unset-environment index aec5ceb3..4fc29b63 100644 --- a/.unset-environment +++ b/.unset-environment @@ -8,8 +8,7 @@ unset HSADMINNG_OFFICE_DATA_SQL_FILE unset HSADMINNG_JWT_ISSUER unset HSADMINNG_JWT_JWKS_URL -unset HSADMINNG_JWT_TOKEN_URL -unset HSADMINNG_JWT_CLIENT_ID -unset HSADMINNG_JWT_CLIENT_SECRET unset HSADMINNG_JWT_USERNAME unset HSADMINNG_JWT_PASSWORD +unset HSADMINNG_JWT_TOKEN_URL + diff --git a/bin/jwt-curl b/bin/jwt-curl index 86f268a0..3caf5993 100755 --- a/bin/jwt-curl +++ b/bin/jwt-curl @@ -102,12 +102,12 @@ function jwtLogin() { # OAuth2 Resource Owner Password Credentials Grant (public client) trace "+ curl --fail-with-body --show-error -X POST \ -H 'Content-Type: application/x-www-form-urlencoded' \ - -d \"grant_type=password&client_id=$HSADMINNG_JWT_CLIENT_ID&client_secret=$HSADMINNG_JWT_CLIENT_SECRET&username=$HSADMINNG_JWT_USERNAME&password=$HSADMINNG_JWT_PASSWORD_DISPLAY\" \ + -d \"grant_type=password&username=$HSADMINNG_JWT_USERNAME&password=$HSADMINNG_JWT_PASSWORD_DISPLAY\" \ $HSADMINNG_JWT_TOKEN_URL -o ~/.jwt-token.response" JWT_RESPONSE=$(curl --fail-with-body --show-error -X POST \ -H 'Content-Type: application/x-www-form-urlencoded' \ - -d "grant_type=password&client_id=$HSADMINNG_JWT_CLIENT_ID&client_secret=$HSADMINNG_JWT_CLIENT_SECRET&username=$HSADMINNG_JWT_USERNAME&password=$HSADMINNG_JWT_PASSWORD_DISPLAY" \ + -d "grant_type=password&username=$HSADMINNG_JWT_USERNAME&password=$HSADMINNG_JWT_PASSWORD" \ $HSADMINNG_JWT_TOKEN_URL 2>&1 | tee ~/.jwt-token.response) # Extract access token from JSON response @@ -198,8 +198,6 @@ case "${1,,}" in "env") ## prints all related HSADMINNG_JWT_... environment variables; use '--show-password' to show the password as well # example: jwt-curl -show-password env echo "export HSADMINNG_JWT_TOKEN_URL=$HSADMINNG_JWT_TOKEN_URL" - echo "export HSADMINNG_JWT_CLIENT_ID=$HSADMINNG_JWT_CLIENT_ID" - echo "export HSADMINNG_JWT_CLIENT_SECRET=$HSADMINNG_JWT_CLIENT_SECRET" echo "export HSADMINNG_JWT_USERNAME=$HSADMINNG_JWT_USERNAME" if [ "$HSADMINNG_JWT_SHOW_PASSWORD" == "yes" ]; then echo "export HSADMINNG_JWT_PASSWORD=$HSADMINNG_JWT_PASSWORD" diff --git a/doc/ideas/login-credentials-data-model.mermaid b/doc/ideas/login-credentials-data-model.mermaid index 0d7e0f43..9bcf775a 100644 --- a/doc/ideas/login-credentials-data-model.mermaid +++ b/doc/ideas/login-credentials-data-model.mermaid @@ -8,11 +8,14 @@ classDiagram Credentials "1..n" --o "1" CredentialsContextMapping class Credentials{ + +totpSecret: text + +phonePassword: text +emailAdress: text +smsNumber: text -active: bool [r/w] -globalUid: int [w/o] -globalGid: int [w/o] + -onboardingToken: text [w/o] } class CredentialsContext{ 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 d834a91e..15687dd0 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsController.java @@ -1,5 +1,7 @@ package net.hostsharing.hsadminng.hs.accounts; +import java.time.LocalDateTime; +import java.time.ZoneOffset; import java.util.List; import java.util.UUID; import java.util.function.BiConsumer; @@ -35,6 +37,7 @@ import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBui import jakarta.persistence.EntityNotFoundException; import jakarta.validation.ValidationException; +import static java.util.Optional.ofNullable; import static java.util.Optional.of; @RestController @@ -188,6 +191,22 @@ public class HsCredentialsController implements CredentialsApi { return ResponseEntity.ok(result); } + @Override + @Transactional + @Timed("app.credentials.credentials.credentialsUsed") + public ResponseEntity credentialsUsed(final UUID credentialsUuid) { + context.define(); + + val current = credentialsRepo.findByUuid(credentialsUuid).orElseThrow(); + + current.setOnboardingToken(null); + current.setLastUsed(LocalDateTime.now()); + + val saved = credentialsRepo.save(current); + val mapped = mapper.map(saved, CredentialsResource.class, ENTITY_TO_RESOURCE_POSTMAPPER); + return ResponseEntity.ok(mapped); + } + private void validateOnCreate(final HsCredentialsEntity newCredentialsEntity) { validateReferencedPersonToBeRepresentedByLoginUserPerson(newCredentialsEntity); validateNormalUsersOnlyAccessPublicContexts(newCredentialsEntity); @@ -305,6 +324,8 @@ public class HsCredentialsController implements CredentialsApi { } final BiConsumer ENTITY_TO_RESOURCE_POSTMAPPER = (entity, resource) -> { + ofNullable(entity.getLastUsed()).ifPresent( + dt -> resource.setLastUsed(dt.atOffset(ZoneOffset.UTC))); of(entity.getSubject()).ifPresent( subject -> resource.setNickname(subject.getName()) ); 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 64ba669b..aa7411fb 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntity.java @@ -9,7 +9,9 @@ import net.hostsharing.hsadminng.repr.Stringify; import net.hostsharing.hsadminng.repr.Stringifyable; // import net.hostsharing.hsadminng.rbac.RbacSubjectEntity; // Assuming RbacSubjectEntity exists for the FK relationship +import java.time.LocalDateTime; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.UUID; @@ -29,6 +31,8 @@ public class HsCredentialsEntity implements BaseEntity, Str protected static Stringify stringify = stringify(HsCredentialsEntity.class, "credentials") .withProp(HsCredentialsEntity::isActive) .withProp(HsCredentialsEntity::getEmailAddress) + .withProp(HsCredentialsEntity::getTotpSecrets) + .withProp(HsCredentialsEntity::getPhonePassword) .withProp(HsCredentialsEntity::getSmsNumber) .quotedValues(false); @@ -47,6 +51,9 @@ public class HsCredentialsEntity implements BaseEntity, Str @Version private int version; + @Column + private LocalDateTime lastUsed; + @Column private boolean active; @@ -56,6 +63,15 @@ public class HsCredentialsEntity implements BaseEntity, Str @Column private Integer globalGid; + @Column + private String onboardingToken; + + @Column + private List totpSecrets; + + @Column + private String phonePassword; + @Column private String emailAddress; 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 af84bc26..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,12 @@ public class HsCredentialsEntityPatcher implements EntityPatcher { + builder.onboardingToken("some-onboarding-token"); + builder.loginContexts(contextRepo.findAll().stream() + .filter(HsCredentialsContext::isPublicAccess).collect(Collectors.toSet())); + }); + + RestAssured // @formatter:off + .given() + .header("Authorization", bearer("superuser-alex@hostsharing.net")) + .port(port) + .when() + .post("http://localhost/api/hs/accounts/credentials/" + credentialsEntity.getUuid() + "/used") + .then().log().all().assertThat() + .statusCode(200) + .contentType("application/json") + .body("uuid", is(credentialsEntity.getUuid().toString())) + .body("onboardingToken", is(nullValue())) + .body("lastUsed", is(not(nullValue()))); + // @formatter:on + } + } + // Helper methods private HsOfficePersonRealEntity givenLegalPerson(final String executingSubjectName) { return jpaAttempt.transacted(() -> { diff --git a/src/test/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntityPatcherUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntityPatcherUnitTest.java index b6d3550b..2f1f69fd 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntityPatcherUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/accounts/HsCredentialsEntityPatcherUnitTest.java @@ -33,11 +33,15 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< private static final Boolean INITIAL_ACTIVE = true; private static final String INITIAL_EMAIL_ADDRESS = "initial@example.com"; + private static final List 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 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"; // Contexts private static final UUID CONTEXT_UUID_1 = UUID.randomUUID(); @@ -98,7 +102,9 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< entity.setUuid(INITIAL_CREDENTIALS_UUID); entity.setActive(INITIAL_ACTIVE); entity.setEmailAddress(INITIAL_EMAIL_ADDRESS); + entity.setTotpSecrets(INITIAL_TOTP_SECRETS); entity.setSmsNumber(INITIAL_SMS_NUMBER); + entity.setPhonePassword(INITIAL_PHONE_PASSWORD); // Ensure loginContexts is a mutable set for the patcher entity.setLoginContexts(new HashSet<>(initialContextEntities)); return entity; @@ -131,12 +137,25 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< PATCHED_EMAIL_ADDRESS, HsCredentialsEntity::setEmailAddress, PATCHED_EMAIL_ADDRESS), + new SimpleProperty<>( + "totpSecret", + CredentialsPatchResource::setTotpSecrets, + PATCHED_TOTP_SECRETS, + HsCredentialsEntity::setTotpSecrets, + PATCHED_TOTP_SECRETS) + .notNullable(), new JsonNullableProperty<>( "smsNumber", CredentialsPatchResource::setSmsNumber, PATCHED_SMS_NUMBER, HsCredentialsEntity::setSmsNumber, PATCHED_SMS_NUMBER), + new JsonNullableProperty<>( + "phonePassword", + CredentialsPatchResource::setPhonePassword, + PATCHED_PHONE_PASSWORD, + HsCredentialsEntity::setPhonePassword, + PATCHED_PHONE_PASSWORD), new SimpleProperty<>( "contexts", CredentialsPatchResource::setContexts, 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 a9b4e9e7..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 @@ -36,8 +36,11 @@ public class CreateCredentials extends BaseCredentialsUseCase "person.uuid": ${Person: %{personGivenName} %{personFamilyName}}, "nickname": ${nickname}, "active": %{active}, + "totpSecrets": @{totpSecrets}, "emailAddress": ${emailAddress}, + "phonePassword": ${phonePassword}, "smsNumber": ${smsNumber}, + "onboardingToken": ${onboardingToken}, "globalUid": %{globalUid}, "globalGid": %{globalGid}, "contexts": @{resolvedContexts} @@ -55,7 +58,9 @@ public class CreateCredentials extends BaseCredentialsUseCase .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 65cfe8f9..5cfd7665 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 @@ -75,7 +75,9 @@ 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("phonePassword", "securePass123") .given("smsNumber", "+49123456789") .given("globalUid", 21011) .given("globalGid", 21011) @@ -83,6 +85,7 @@ class CredentialsScenarioTests extends ScenarioTest { "contexts", Array.of( Pair.of("HSADMIN", "prod") )) + .given("onboardingToken", "fake-unboarding-token") .doRun() .keep(); } @@ -96,7 +99,9 @@ class CredentialsScenarioTests extends ScenarioTest { .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( 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 index d665e6c8..4714bc1e 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/UpdateCredentials.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/accounts/scenarios/UpdateCredentials.java @@ -27,12 +27,16 @@ public class UpdateCredentials extends BaseCredentialsUseCase 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; @@ -45,7 +49,8 @@ public class UpdateCredentials extends BaseCredentialsUseCase () -> httpGet("/api/hs/accounts/credentials/%{credentialsUuid}") .expecting(OK).expecting(JSON), path("uuid").contains("%{newCredentials}"), - path("nickname").contains("%{nickname}") + path("nickname").contains("%{nickname}"), + path("totpSecrets").contains("%{totpSecrets}") ); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java index 911603a3..ad3496c2 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java @@ -31,12 +31,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; +@Transactional @Tag("officeIntegrationTest") @SpringBootTest( webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, classes = HsadminNgApplication.class) @ActiveProfiles("fake-jwt") -@Transactional class HsOfficeBankAccountControllerAcceptanceTest extends ContextBasedTestWithCleanup { @LocalServerPort